Skip to content

[concurrency] Change #isolated to mask out the TBI bits of the witness pointer of the implicit isolated any Actor pointer so we can do optimizations on TBI supporting platforms in the future. #83346

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 10 commits into
base: main
Choose a base branch
from

Conversation

gottesmm
Copy link
Contributor

Specifically, this will let us perform an optimization in the future where a caller can set bits in the TBI byte to signal information dynamically to a nonisolated(nonsending) callee. The intent is that this will let us signal to a nonisolated(nonsending) callee that the caller isn't performing any isolated work before it hops (or potentially a caller of the caller) hops meaning that the nonisolated(nonsending) function can avoid a tail hop that restores isolation after calling some sort of actor isolated call.

E.x.:

nonisolated(nonsending) func callsActor(_ a: MyActor) async {
  await a.doSomething()
}

@concurrent func runOnBackground() async {}

actor A {
  func doSomething() async { ... }

  func caller() async {
    await callsActor(self)
    await runOnBackgroud()
  }
}

In the above case, the hop back in callsActor is not needed dynamically in caller since we are going to immediately await ontot he background. But we do not know that. An optimization can be written that can recognize this case and dynamically tell via the tbi bits of the protocol witness table of the any Actor call that the hop to executor call can avoid the hop.

Since this only applies to the implicit isolated any Actor parameter which is only used for the purpose of hopping, it is safe to do this since when we hop we just call the Actor unowned executor dispatch thunk which directly accesses the witness table by loading from that pointer... meaning that anything set in the TBI bit is irrelevent.

The only other way to grab this implicit bit is to use #isolation and that is where this commit comes into play. Since we can access the actor via #isolation, we need to ensure that we mask out the TBI bits so that the rest of the runtime/compiled code never see the TBI bits. This is because we do not want the fact that we are using these TBI bits to ever escape into other code (e.x.: consider things that may cast).

So we have basically created a hermetically sealed world where we can pass information from callee to caller using the protocol witness TBI bits.

rdar://156525771

@gottesmm
Copy link
Contributor Author

@swift-ci smoke test

@gottesmm gottesmm force-pushed the pr-df6aafa41c61b9a9a6ee4965a1564ec2946a0dc9 branch from 1fa6f01 to f770390 Compare July 31, 2025 22:39
@gottesmm
Copy link
Contributor Author

@swift-ci smoke test

Copy link
Contributor

@ktoso ktoso left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AFAICS looking good 👍

@gottesmm gottesmm force-pushed the pr-df6aafa41c61b9a9a6ee4965a1564ec2946a0dc9 branch 2 times, most recently from f589761 to dcecf88 Compare August 1, 2025 19:16
@gottesmm gottesmm requested a review from AnthonyLatsis as a code owner August 1, 2025 19:16
@gottesmm
Copy link
Contributor Author

gottesmm commented Aug 1, 2025

@swift-ci smoke test

@gottesmm
Copy link
Contributor Author

gottesmm commented Aug 9, 2025

John asked me to change how I did this to add a new type. To make it easier, I have redone the patch without inserting the bit clearing so I can test what should be NFC from the programmers perspective (inserting the new type). I am going to test that now and then add back in the bit clearing once I get a green on that from a test perspective. So I am going to push and we are going to appear to lose the bit clearing stuff.

@gottesmm gottesmm force-pushed the pr-df6aafa41c61b9a9a6ee4965a1564ec2946a0dc9 branch from dcecf88 to 458f3b5 Compare August 9, 2025 01:58
@gottesmm
Copy link
Contributor Author

gottesmm commented Aug 9, 2025

... but I am going to add back in the bit clearing stuff later.

@gottesmm
Copy link
Contributor Author

gottesmm commented Aug 9, 2025

@swift-ci smoke test

…inTypeKind.

This is an NFC change. The idea is that this will make it easier to add new
BuiltinTypeKinds. I missed this code when adding Builtin.ImplicitIsolationActor.
By adding a covered switch, we can make sure this code is updated in the
future.
…changing the pass.

Specifically, I am refactoring out the code that converts actor/Optional<any
Actor> to an executor in preparation for adding code to LowerHopToExecutor that
handles Builtin.ImplicitIsolationActor.

The only actual functional change is that I made getExecutorForOptionalActor
support being invoked when generating code (i.e. when its SILBuilder has an
insertion point at the end of the block). It previously assumed that it would
always have a real SILInstruction as an insertion point. The changes can be seen
in the places where we now check if the insertion point equals the end of a
block. Its very minor and due to conditional control flow doesn't have any
actual impact given the manner that the code today is generated. This came up in
a subsequent commit when I reuse this code to generate a helper function for
converting Builtin.ImplicitIsolationActor to Builtin.Executor.
This is currently not wired up to anything. I am going to wire it up in
subsequent commits.

The reason why we are introducing this new Builtin type is to represent that we
are going to start stealing bits from the protocol witness table pointer of the
Optional<any Actor> that this type is bitwise compatible with. The type will
ensure that this value is only used in places where we know that it will be
properly masked out giving us certainty that this value will not be used in any
manner without it first being bit cleared and transformed back to Optional<any
Actor>.
…piling for a triple that supports AArch64 TBI.

Just breaking down layers of a larger patch to make it easier to review.
…ding) by default enabled.

Just adding more code coverage.
…tional<any Actor> as an expected executor.

We want SILGen to have a simplified view of its executor and know that whenever
one sees an Actor, it is an actual actor instead of a Builtin.Executor. This
just simplifies code. Also, we should eventually have an invariant that
Builtin.Executor should only be allowed in LoweredSIL after LowerHopToExecutor
has run. But that is a change for another day.
…ctor> to represent the implicit isolated parameter.

NOTE: We are not performing any bitmasking at all now. This is so that we can
transition the code base/tests to expect Builtin.ImplicitIsolationActor instead
of Optional<any Actor>.

NOTE: The actual test changes are in the next commit. I did this to make it
easier to review the changes.

This should not have any user visible changes.
@gottesmm gottesmm force-pushed the pr-df6aafa41c61b9a9a6ee4965a1564ec2946a0dc9 branch from 458f3b5 to a5caee1 Compare August 12, 2025 02:11
@gottesmm
Copy link
Contributor Author

@swift-ci smoke test

swiftlang/llvm-project#11157

@gottesmm gottesmm force-pushed the pr-df6aafa41c61b9a9a6ee4965a1564ec2946a0dc9 branch from a5caee1 to 97cb71e Compare August 12, 2025 03:23
@gottesmm
Copy link
Contributor Author

@swift-ci smoke test

swiftlang/llvm-project#11157

1 similar comment
@gottesmm
Copy link
Contributor Author

@swift-ci smoke test

swiftlang/llvm-project#11157

@gottesmm
Copy link
Contributor Author

swiftlang/llvm-project#11157
@swift-ci smoke test

@gottesmm
Copy link
Contributor Author

I want JohnM to take a look at this. So I am going to wait on merging.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants